The CI failure is that my new inherent methods interrupted access to the extension trait methods in rayon-future's tests -- meh.

@alexcrichton, @fitzgen, maybe you have some perspective from the WASM side? Would either of these signatures be better or worse for integrating with wasm/js futures?

• spawn_future(impl FnOnce) -> impl Future
• spawn_future(impl Future) -> impl Future

I suspect the latter might be problematic, having to wait for unknown input futures from rayon threads, since we can't really unwind the entire thread out of wasm.

But is the simpler FnOnce->Future feasible in that environment? e.g. Will it work for the rayon thread to call a Waker that ultimately notifies the javascript stuff?

I believe that should work and there aren't any technical restrictions on our side (although Alex knows more about our executor in a multithreaded context).

My one concern, unrelated to wasm, is using -> impl Future means that we can't put the result in a struct without boxing into a trait object. Is there a technical reason why it can't be a named type?

@cuviper

Definitely rayon is "throughput optimized" and may not be the best choice to use for all your futures. But where I thought the impl Future -> impl Future signature could be useful is creating a DAG of computation nodes that are interdependent. Under this setup, one could e.g. have a matrix of tasks each task is able to .await other tasks. (Although I guess i'd be a bit of a pain to set it up -- interesting problem.)

In any case, I also don't think it causes a big problem if you block on I/O events in rayon. The future would simply not be in our thread-pools until the I/O event was "wakened". It's not like it would block in the normal sense. (You might not want to combine that with rayon::scope, I suppose.)

@fitzgen

My one concern, unrelated to wasm, is using -> impl Future means that we can't put the result in a struct without boxing into a trait object. Is there a technical reason why it can't be a named type?

Just that it's a smaller API surface to consider in this PR if we don't name it. But the type is simple enough that it shouldn't be a problem to return the concrete type.

@nikomatsakis

In any case, I also don't think it causes a big problem if you block on I/O events in rayon. The future would simply not be in our thread-pools until the I/O event was "wakened".

Yeah, so this is a case where I'm not certain how these things would work. I guess if the async work leading up to our part isn't ready, we'd just return NotReady too and let some outer executor deal with it? AIUI in async-task, we'd just end up queuing an underlying spawn for each step of the future's progress.

I brought up the WASM case because I thought there were some big caveats about notifying WASM threads about javascript futures. But maybe if the actual task suspension still looks like it happens outside of the threadpool, and we're respawning to get back in, it may work OK? Not sure.

We should probably play with some real examples before we commit to anything.

As to the effect on wasm I think the main question here would be what extra APIs rayon needs from the host. For example in our demos we have a Pool structure which only supports a run method to run a closure on the pool, and then we use rayon's ThreadPoolBuilder to use that which allows us to schedule rayon's work on our own thread pool we manage.

If impl Future is the argument here then rayon would probably need to add another API along the lines of "please schedule this future work to happen on the rayon thread pool" or something like that? If not though then I don't think this really impacts wasm all that much! If you've got a demo of the API though I can poke around the code and see if anything looks like it'd break on wasm (or try to run the demo on wasm!)

An update:

So we talked at some length to @alexcrichton and @fitzgen on Discord and came to the conclusion that "WASM interop is not a major issue here".

We left the meeting with the conclusion that I would spend a bit of time looking into what it would take to implement a spawn_future(impl Future) -> impl Future approach. I wanted to give a brief update on that.

(Side note: I know this will shock absolutely no one, but I think this is worth moving to an RFC. There are a number of smaller details concerning the APIs worth talking over and documenting. )

The set of APIs I think we want to support are something like this:

• spawn a future into the global scope -- spawn_future(impl Future + 'static) -> impl Future
• spawn a future into a Rayon scope -- spawn_future(impl Future + 'scope) -> impl Future

(And probably that impl Future in the return type is really going to be RayonFuture.)

Anyway, I dug into what an implementation would take. I think if we were to reimplement everything ourselves, the primary thing we need to create is a Waker. The idea would roughly be like this:

• When you spawn a future, we create an Arc to represent that job
  • This Arc implements the Rayon Job trait, so it can enqueued in our thread-pools
  • This Arc also serves as the Waker
  • This Arc also serves as the resulting future
  • If launched inside of a Rayon scope, this Arc also starts out holding a ref-count on that scope, which prevents the scope from completing until the future has fully executed. This is what allows it to hide the 'scope lifetime.
• The arc has an internal state machine that looks something like this:
  • Waiting(F) -- initial state. We have the future but are not enqueued in Rayon task pool.
  • Enqueued(F) -- enqueued in Rayon task pool.
  • Executing -- currently executing the future
  • PendingWake -- if a wake occurred while we were executing
  • Complete(F::Result) -- future completed
• There are three ways to interact with this arc:
  • As a waker: when the "wake" method is called, we can be in any state. We transition as follows:
    • If Waiting, go to Enqueued state and enqueue ourselves into a Rayon thread-pool.
    • Otherwise, ignore.
  • As a Rayon job: when the rayon job executes, we must be in the enqueued state.
    • Move to Executing state and execute future.
    • If the result is "NotReady", move to the Waiting state, unless we were moved to PendingWake, in which case we can immediately re-execute (or re-enqueue ourselves)
    • If the result is Ready, move to the Complete state and store the result
  • As a future, we can be polled:
    • If not in the complete state, we return NotReady, and stash the waker
    • Else we return the result ("take"ing it)
    • Upon entering the complete state, we wake the waker (this actually occurs in the "job" code)

I started writing this code, but it turns out that the async-task crate basically implements all of this logic already, so it really makes sense I think to build on that.

The simplest integration (at the static level, and not "peak efficiency") just looks like this:

pub fn spawn_future<R>(future: impl Future<Output = R> + Send + 'static) -> impl Future<Output = R>
where
    R: Send + 'static,
{
    let (task, handle) = async_task::spawn(
        future,
        move |task| rayon_core::spawn(move || task.run()),
        (),
    );
    task.schedule();
    async move { handle.await.unwrap() }
}

I saw "not peak efficiency" because each call to rayon_core::spawn is going to create a new allocation, but the async_task::spawn already created one behind the scenes.

I discussed with @stjepang -- if async_task::spawn offered an API for converting from a async_task::Task to some pointer (analogous to Arc::into_raw), then we could do something like this:

• convert the task to a raw pointer *const TaskRaw or whatever
• implement rayon::Job for TaskRaw (Job is a private trait, so we can do this without the outside world knowing about it, but it does have to be in rayon-core presently)
• enqueue the task directly onto the rayon thread-pool; when invoked, it'll use from_raw to convert the raw pointer back and then invoke run as above

To handle the 'scope API, we need to ensure that async-task meets some basic sanity requirements. But if we are doing this development within rayon-core, we don't need to expose any new APIs publicly. We would basically follow the approach I suggested above, I think -- we would grab a ref count and then augment the future we are spawning to decrement the ref count only when its completed its work. I didn't get so far as to prototype this, in part because I was doing this work in a separate crate. I realize now that I could prototype this better if I did the work directly in rayon-core (though we should discuss if this is a feature we would want in rayon-core, as it would up our minimum rust version -- perhaps gated by a cargo feature?).

(though we should discuss if this is a feature we would want in rayon-core, as it would up our minimum rust version -- perhaps gated by a cargo feature?)

I think it should be in core. It doesn't have to affect the general minimum rust if we gate the new functionality, whether by a cargo feature or just autocfg detection, as I did here for Future.

I'm about halfway through a straightforward port of the existing rayon_futures to 0.3, without any thought into changing the API design beyond removing Future::wait.

The library builds and feels approximately correct but I haven't ported the tests yet, thus about halfway. I think the most interesting part of that work has been how much simpler ArcWake is than the previous Notify stuff.

I don't have too many opinions about what the API should be. I do think that dropping a Future should be considered a perfectly normal way to cancel it. That likely means we need to take a bit more care to propagate panics in a useful way.

Any progress on this, for someone with a mix of CPU / IO bound work what's the recommended path?

I haven't have the time lately to work on the more advanced ideas proposed, nor do I really have any expertise in async code.

I still think the very simple idea in this PR, FnOnce to Future, would be useful for simple use cases though. @jannickj, could you try this branch in your code and see how that works for you?

@cuviper I'm also interested in being able to ergonomically offload compute-bound work (or possibly work that just still has to be done with blocking IO) in an async context. Thanks for working on this!

My feeling is that the API that @nikomatsakis proposed is probably the right one because compute bound work might want to interleave some async IO operations and a future captures that sort of computation better than a closure. That said, for now a closure-based API is good enough for me.

To hold me over while you all take the time to design a nice interface, I just threw together https://github.com/ethanpailes/rayon-future. Maybe it will be useful to someone else in the same boat.

I noticed that the new gtk-rs release includes a threadpool with push and push_future methods. The latter is like what I proposed here, just mapping a closure to a future.

pub fn push_future<T: Send + 'static, F: FnOnce() -> T + Send + 'static>(
    &self, 
    func: F
) -> Result<impl Future<Output = T>, Error>

I still like the simplicity of not trying to await future input on our part. It's also possible for us to add that in the short term, and maybe add a separate method that maps futures in&out later.

Hmm, I think I expected spawn_future to take a future as argument,

Revisiting this -- maybe the answer is to give a different name to this fn(impl FnOnce) -> impl Future?

The tokio-rayon crate provides this with a top-level spawn function, but they also have an extension trait for ThreadPool where they call it spawn_async, which seems like a decent name to use in rayon-core too. Then we can leave the door open for a spawn_future that takes a future as input.

Gentle poke here, there's renewed interest on Bevy's side in (re-)adopting rayon for thread pool management and task execution, particularly due to its better thread sleep management than our current use of async_executor. One of the biggest reasons we initially moved away from rayon is because of the inability to execute both async and non-async tasks in the same thread pool to try to minimize OS context switch costs where possible.

Is there still interest in supporting this use case? If so, what's needed to advance this forward? There was mention of needing an RFC, but there doesn't seem to be a PR for one in the RFCs repo, open or closed.

Right now we can complete a migration to using rayon-core and just use @nikomatsakis's non-"peak efficiency" implementation mentioned above, but it'd be desirable to eliminate the extra allocation on re-scheduling the runnable.

I think it could be built on something like this, if you want to experiment with it:

impl JobRef {
    pub(super) fn new_async(runnable: async_task::Runnable<()>) -> JobRef {
        // TODO: call registry.increment_terminate_count()?
        JobRef {
            pointer: runnable.into_raw().as_ptr(),
            execute_fn: |this| unsafe {
                // TODO: handle panics with registry.catch_unwind(..)
                // TODO: should we always use NonNull for `Job`?
                // TODO: call registry.terminate()?
                let this = NonNull::new_unchecked(this as *mut ());
                async_task::Runnable::<()>::from_raw(this).run();
            },
        }
    }
}

(I don't think there's any value in literally implementing Job for this.)